-
Notifications
You must be signed in to change notification settings - Fork 41
Fix handling of entity with class names without "Entity" or "Entities" #75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. Can I OCD over a nit please? Sorry!
expose :nested, using: TheseApi::Entities::Nested, documentation: { desc: 'Nested object.' } | ||
expose :nested_child, using: TheseApi::Entities::NestedChild, documentation: { desc: 'Nested child object.' } | ||
expose :polymorphic, using: TheseApi::Entities::Polymorphic, documentation: { desc: 'Polymorphic Model' } | ||
expose :mixed, using: TheseApi::Entities::MixedType, documentation: { desc: 'A model with mix of types' } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expose :mixed, using: TheseApi::Entities::MixedType, documentation: { desc: 'A model with mix of types' } | |
expose :mixed, using: TheseApi::Entities::MixedType, documentation: { desc: 'A model with mix of types.' } |
(and let's replace Polymorphic Model with A polymorphic model.
above, to be consistent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done here: 47e6140
Of course! All suggestions are always welcome. |
strategy: | ||
fail-fast: false | ||
matrix: | ||
ruby-version: ['3.0', '3.1', '3.2', '3.3'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm.. Maybe instead of removing ruby 3.0, the specific grape version can be set instead. As a separate PR ofc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea, will merge this one, feel free to pick it up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! Will do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As promised: #76
This commit fixes an issue where specifying a class like `Time` for the `type` option in `expose` documentation resulted in an error instead of generating the correct Swagger/OpenAPI specification. This functionality was working correctly in v0.5.5 but regressed after PR ruby-grape#75, leading to the following error: ``` GrapeSwagger::Errors::UnregisteredParser: No parser registered for Time. ``` The `primitive_type?` method now uses `GrapeSwagger::DocMethods::DataType.call` to resolve the type name before checking if it's a primitive type. This ensures that types like `Time` are correctly identified as strings with the `date-time` format, restoring the previous behavior.
This commit fixes an issue where specifying a class like `Time` for the `type` option in `expose` documentation resulted in an error instead of generating the correct Swagger/OpenAPI specification. This functionality was working correctly in v0.5.5 but regressed after PR ruby-grape#75, leading to the following error: ``` GrapeSwagger::Errors::UnregisteredParser: No parser registered for Time. ``` The `primitive_type?` method now uses `GrapeSwagger::DocMethods::DataType.call` to resolve the type name before checking if it's a primitive type. This ensures that types like `Time` are correctly identified as strings with the `date-time` format, restoring the previous behavior.
Improves the handling of unconventional entity class names and enhances support for diverse types in Swagger documentation generation.
Changes
ambiguous_model_type?
to correctly identify entities with non-standard class names.primitive_type?
for clearer handling of primitives and additional non-class types (string, array).